Repository navigation
OSAC-3160: Add GPU columns to InstanceType CLI table rendering - #128
omer-vishlitzky merged 4 commits into
Conversation
|
@Tzif-Morgen: This pull request references OSAC-3160 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: osac-project/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughInstance type tables now include GPU count and resource name columns. Tests cover GPU-enabled instances and fallback values for instances without GPU configuration. ChangesGPU instance type table rendering
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@fulfillment-service/internal/rendering/table_renderer_test.go`:
- Around line 202-250: Extend the “Optional GPU columns” coverage around
renderInstanceTypes to exercise the private InstanceType table as well as the
existing public type. Add equivalent GPU-enabled and non-GPU cases that verify
configured GPU values and the 0/- fallback output, or parameterize the test
helper to accept both API types while preserving the current assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 72ec578d-b431-44c8-8211-0bc29c7274ce
📒 Files selected for processing (3)
fulfillment-service/internal/rendering/table_renderer_test.gofulfillment-service/internal/rendering/tables/osac.private.v1.InstanceType.yamlfulfillment-service/internal/rendering/tables/osac.public.v1.InstanceType.yaml
eb79a69 to
082c33e
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Tzif-Morgen, ygalblum The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Tzif <tmorgens@redhat.com>
Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Tzif <tmorgens@redhat.com>
… NAME Remove the GPU DEVICE column (pci_device_selector) from the InstanceType table — not useful at a glance. Rename GPU COUNT to GPUs and GPU RESOURCE to GPU NAME for clarity. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Tzif <tmorgens@redhat.com>
Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Tzif <tmorgens@redhat.com>
082c33e to
281b280
Compare
|
/lgtm |
…-page OSAC-3604: Storage Tiers list page
OSAC-3160: Add GPU columns to InstanceType CLI table rendering
Jira: OSAC-3160
Story type: [DEV]
Summary
Adds GPUS and GPU NAME columns to the InstanceType CLI table output (both public and private APIs), so users can identify which InstanceTypes include GPU hardware at a glance. Uses CEL
has()guards to safely render blank values for non-GPU InstanceTypes.Changes
internal/rendering/tables/osac.public.v1.InstanceType.yaml— Added GPUS, GPU NAME columns between MEMORY and STATEinternal/rendering/tables/osac.private.v1.InstanceType.yaml— Same GPU columns for the private/admin APIinternal/rendering/table_renderer_test.go— Added "Optional GPU columns" test block verifying GPU-enabled and non-GPU InstanceType rendering; hoisted sharedrenderInstanceTypeshelperExample Output
Testing
has()guards for both public and private YAMLsAcceptance Criteria
0/-in GPU columnsSummary by CodeRabbit
0and-placeholders when GPU information is unavailable.